Skip to content

fix: clamp negative line numbers when reading files #2337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mrubens merged 1 commit into from
Apr 6, 2025
Merged

fix: clamp negative line numbers when reading files #2337

mrubens merged 1 commit into from
Apr 6, 2025

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented Apr 6, 2025 •
edited by ellipsis-dev bot
Loading

Context

This PR improves the handling of line numbers in the readLines function to be more forgiving with AI-generated inputs.

Implementation

  • Clamp negative startLine values to 0 instead of throwing errors
  • Convert non-integer line numbers to integers automatically using Math.floor()
  • Only reject when line numbers aren't numbers at all
  • Update tests to match new behavior

These changes prevent unnecessary model retries by handling imperfect AI-generated line numbers gracefully, making the system more robust when interacting with AI models.

How to Test

  • Run the updated tests which verify the new behavior
  • Try using the readLines function with negative or non-integer line numbers

Get in Touch

Discord: KJ7LNW

Important

Enhances readLines to handle negative and non-integer line numbers by clamping and flooring, updating tests accordingly.

  • Behavior:
    • readLines in read-lines.ts now clamps negative startLine to 0 and floors non-integer line numbers.
    • Rejects only if line numbers are not numbers.
    • Updates error handling to ensure startLine is less than or equal to endLine.
  • Tests:
    • Updated tests in read-lines.test.ts to reflect new behavior, including clamping and flooring of line numbers.
    • Added tests for negative and non-integer line numbers.

This description was created by Ellipsis for 63659a6. It will automatically update as commits are pushed.

fix: clamp negative line numbers when reading files
63659a6 
Improve handling of AI-generated line numbers:
- Clamp negative startLine values to 0 instead of throwing errors
- Convert non-integer line numbers to integers automatically
- Only reject when line numbers aren't numbers at all

These changes prevent unnecessary model retries by handling imperfect
AI-generated line numbers gracefully, making the system more robust
when interacting with AI models.

Signed-off-by: Eric Wheeler <[email protected]>
@KJ7LNW KJ7LNW requested review from cte and mrubens as code owners April 6, 2025 02:01
@changeset-bot
Copy link

changeset-bot bot commented Apr 6, 2025

⚠️ No Changeset found

Latest commit: 63659a6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Apr 6, 2025
mrubens
mrubens approved these changes Apr 6, 2025
View reviewed changes
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really include non-integer line numbers? Crazy...

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 6, 2025
@mrubens mrubens merged commit 5e0c9a7 into RooCodeInc:main Apr 6, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Apr 6, 2025
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Apr 6, 2025

Does it really include non-integer line numbers? Crazy...

I have not seen it do that but the test is there just in case. this does fix the issue where the model may try to use line number zero, which gets adjusted to an index (line-1) causing it to go negative, so the biggest fix is clamping lines <0 to be ==0 instead of throwing an error.

SmartManoj pushed a commit to SmartManoj/Raa-Code that referenced this pull request May 6, 2025
@0xToshii @saoudrizwan
encapsulate ContextManager (RooCodeInc#2337)
88c33ee 
Co-authored-by: Saoud Rizwan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrubens mrubens mrubens approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

Assignees

No one assigned

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KJ7LNW @mrubens